refactor(TAA): readable variable names + named constants#90
Conversation
|
Warning Review limit reached
More reviews will be available in 29 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
No actionable suggestions for changed features. |
alandtse
changed the title
alandtse
changed the title
alandtse
force-pushed
the
refactor/taa-readable-vars
branch
from
c5d6cdb to
abc26a0
Compare
There was a problem hiding this comment.
Pull request overview
Readability-focused refactor of the ISTemporalAA.hlsl decompile transcription, aiming to keep the shader behavior/compiled output unchanged while making the code easier to follow and maintain.
Changes:
- Introduces named constants for repeated “magic” literals used in luma clamping/thresholding and similarity scaling.
- Replaces repeated alpha-coverage gating statements with a compact unrolled loop.
- Renames key intermediate values (e.g., center/neighbor luma/color, motion metrics, flicker/history blend factors) to better communicate intent and register lifetimes.
|
|
||
| static const float3 kLumaWeights = float3(0.5, 0.25, 0.25); | ||
|
|
||
| // Named magic constants from the vanilla decompile (values unchanged). |
alandtse
force-pushed
the
refactor/taa-readable-vars
branch
from
abc26a0 to
2e5df5f
Compare
alandtse
force-pushed
the
refactor/taa-readable-vars
branch
from
2e5df5f to
a02a2f9
Compare
|
|
||
| static const float3 kLumaWeights = float3(0.5, 0.25, 0.25); | ||
|
|
||
| // Named magic constants from the vanilla decompile (values unchanged). |
|
Consolidated into #98, which now targets dev directly with the full ISTemporalAA refactor (readable vars + core restructure + register-pack destructure). |
Behavior-preserving readability pass on the
ISTemporalAA.hlsldecompile transcription. Supersedes #85 (its constants + alpha-loop commits are included here) — folded into one cohesive PR.What changed (all bytecode-identical)
kMaxLumaCap,kMinLumaCap,kLumaEpsilon,kSimilarityScale.AlphaCoverageMask(...) ? x : 0lines.centerLuma,centerColor,neighborColor,motionLength,motionConfidence,motionNormScale,feedbackWeight,depthMaskPass,allTransparent,centerCoverage,uvMinCoverage,uvMinBelowHist,outputLuma,resolved,flickerScore,historyBlend,clampToNeighborhood,reject.Proven safe
Every step gated by
tools/verify-shader-refactor.ps1(#86): compiled DXBC is byte-identical (SHA-256) to the pre-refactor revision across all four permutations (flat / VR / HDR / VR+HDR). No math, swizzle, control-flow, or#ifdefchanges. Also deployed + spot-checked in-game.Out of scope (Standard B, separate follow-up)
The dense bracket-selection cascade and flicker bubble-passes stay as register packs — restructuring those into helpers/loops diverges bytecode and needs runtime A/B validation; tracked separately, ideally upstreamed.
🤖 Generated with Claude Code